Just a quick drive-by comment, I was the one who originally started this transition and was personally responsible for the Gutenberg PR. I managed to replace the dedupe usages without much complex logic or the new dependency you're using here. The code affected seems very similar, probably copy-pasted. I suggest you take a look at what I did there in case it helps you simplify this PR and achieve better consistency.

Note that I don't remember the exact approach I took, but I might have decided to simply not dedupe at all in some cases, since it doesn't really affect any functionality and the trade-off of doing the computation is probably not worth it in the first place.

Just a suggestion :) thanks for applying this transition here <3

but I might have decided to simply not dedupe at all, since it doesn't really affect any functionality

You may want to double-check that.

$ node -e 'const classnames = require( "classnames" ); console.log( classnames( "aa bb cc", { bb: false } ) );'
aa bb cc
$ node -e 'const clsx = require( "clsx" ); console.log( clsx( "aa bb cc", { bb: false } ) );'
aa bb cc
$ node -e 'const classnames = require( "classnames/dedupe" ); console.log( classnames( "aa bb cc", { bb: false } ) );'
aa cc

Ah, yep I replaced that behavior with a very very simple alternative. If you dig in my PR you'll find my approach. With that sentence in my comment I meant cases where the resulting class name is something like "class class".

Glancing at your PR, the following seem likely to be broken to me

• https://github.com/WordPress/gutenberg/blob/200f7c7a210c35eb9c004da9a3e4f1062f471d57/packages/block-library/src/cover/edit.native.js#L301-L303
• https://github.com/WordPress/gutenberg/blob/200f7c7a210c35eb9c004da9a3e4f1062f471d57/packages/block-library/src/embed/embed-preview.js#L119-L121

Also, in https://github.com/WordPress/gutenberg/blob/200f7c7a210c35eb9c004da9a3e4f1062f471d57/packages/block-library/src/embed/util.js#L191-L193 you might mangle things if a class in outputClassNames has one of the classes in aspectRatioClassNames as a substring. For example, pass in "foo wp-has-aspect-ratio-xxx bar", you'll get back "foo -xxx bar".

I managed to replace the dedupe usages without much complex logic or the new dependency you're using here.

The one dedupe instance here required negation (removal of various classes). We don't use clsx or classnames for it. I'm not sure what new dependency you're referring to?

The code affected seems very similar, probably copy-pasted. I suggest you take a look at what I did there in case it helps you simplify this PR and achieve better consistency.

I can't speak to the origin of the code, but in this case it was an independent refactor; looking now at that other PR it seems the goal is slightly different, but luckily it isn't too complex!

Internal announcement: pdWQjU-Mt-p2